[Lang] [refactor] Add Field classes for ti.field/ti.Vector.field/ti.Matrix.field #2638
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/format |
Updated. |
k-ye
approved these changes
Aug 6, 2021
victoriacity
approved these changes
Aug 6, 2021
There was a problem hiding this comment.
LGTM! This will make #2627 much clearer than what it is now. It will also facilitate the implementation of recursive custom structs.
|
/format |
|
P.S. It looks like taichi_glsl defines extensions of the current Matrix/Expr classes. I'm not sure if the package will still be fully functional after the changes are merged. |
Thanks for catching this! Let's get this in first to move other things forward :-) |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue = close #2499
Previously, a
ti.fieldis aExprwithis_global()=True, while ati.Matrix.fieldis aMatrixwithis_global()=True. This is not intuitive. After this PR,ti.fieldwill become aScalarField, andti.Matrix.fieldwill become aMatrixField, which both inherit from a super classField.The
Fieldclass keeps the original implementation way of using SNodes. A field is constructed by a list of field members. For example, a scalar field has 1 field member, while a 3x3 matrix field has 9 field members. A field member is a PythonExprwrapping a C++GlobalVariableExpression. A C++GlobalVariableExpressionwraps the corresponding SNode.All methods belonging to
ti.field/ti.Matrix.fieldhave been migrated fromExpr/Matrixclasses toScalarField/MatrixFieldclasses. If a method is implemented exactly the same in both classes, it will be put intoFieldclass. Otherwise common methods will be marked asNotImplementedinFieldclass to avoid misuse.During migration, the property methods and meta (fill, to/from numpy/torch, copy) methods are mainly kept the same with slight cleaning in both implementation and docstrings. The biggest change is around subscript (both in Python scope and Taichi scope) - the main idea is that for
MatrixField, after first subscript with coordinates, we should get aMatrix, where each element is under the same state as if we are accessing a correspondingScalarField. It acts just like aMatrixso no moreProxyclasses are needed. By the way,Matrix.with_entries()is just added for this purpose.Note:
xis a field with 0-D shape, although deprecated, it can still be accessed viaxinstead ofx[None]. After this PR this invalid behavior will not work any more.